ci: enforce typecheck and version sync - #59
Merged
Conversation
esbuild strips types without checking them, so neither `build` nor `test` (which runs with TS_NODE_TRANSPILE_ONLY=1) could catch a type error. Nothing in CI ran tsc, so a type regression could have shipped. Adding that gate surfaced three pre-existing errors in the test project: - Two TS5097: tests import with explicit .ts extensions so ts-node/ESM can resolve them at runtime. Enabled allowImportingTsExtensions + noEmit in tsconfig.test.json so tsc accepts the same paths. - One TS2741: NoteStats.lastOpened is declared required, but metrics.ts falls back to mtime when it is absent (SPEC 3.2). store.ts guarantees the field post-sanitization, so the production invariant is correct and the test was constructing a deliberately degraded record. Fixed in the test with a cast rather than weakening the type, which would have made store.ts's guarantee meaningless. Also adds scripts/check-version-sync.mjs to enforce the release invariants CLAUDE.md previously documented as manual: manifest/package version parity, a versions.json entry mapping to minAppVersion, and (on release) tag equality. Obsidian's plugin store matches the tag against manifest.json as a plain string, and `on: push: tags: ["*"]` fired for any tag with nothing validating it. CI runs the check without the tag arg so a mis-bump fails at PR time; the release workflow runs it first, before npm ci, so a mistyped tag fails in seconds. No release artifact changes: main.js, styles.css and manifest.json are byte-identical to the published 0.5.0 (main.js sha256 561dc75d…), so no version bump or re-release is required. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two gaps in an otherwise tight release pipeline:
npm testruns withTS_NODE_TRANSPILE_ONLY=1. No workflow rantsc, so a type regression could ship.release.ymlfires ontags: ["*"], andCLAUDE.mddocuments that the tag MUST equalmanifest.jsonversion because Obsidian's store matches them as plain strings — but that invariant was purely manual, as were thepackage.jsonparity andversions.jsonentry.What changed
npm run typecheck(tsc --noEmitover both tsconfigs), wired into CI and release.scripts/check-version-sync.mjsenforcing manifest/package parity, aversions.jsonentry matchingminAppVersion, and — when given a tag — tag equality.release.ymlruns it first, beforenpm ci; it is dependency-free, so a mistyped tag fails in seconds.CLAUDE.mdupdated — it described these as manual steps.Pre-existing type errors this surfaced
Adding the gate turned it red, which was the point:
.tsextensions for ts-node/ESM resolution. Fixed withallowImportingTsExtensions+noEmitintsconfig.test.json.NoteStats.lastOpenedis declared required, butmetrics.tsdoesstats.lastOpened ?? fallbackMtime ?? nowand SPEC.md documents the mtime fallback.store.tsguarantees the field post-sanitization, so the production invariant is correct and the test was constructing an intentionally degraded record. Fixed in the test with a cast rather than making the field optional, which would have madestore.ts's guarantee meaningless.No release required
main.js,styles.cssandmanifest.jsonare byte-identical to the published 0.5.0 (main.jssha256561dc75d…). Everything here is dev tooling; nothing ships to users, so no version bump.Verification
check:versions,lint,typecheck,build,testall pass locallyrelease.yml, not retyped0.5.0and the no-tag CI form; fails onv0.5.0, package/manifest drift, missingversions.jsonentry, andminAppVersionmismatch🤖 Generated with Claude Code